fix(utils): allow negative POSIX timestamps in from_timestamp() - #3020
Closed
Sohel2309 wants to merge 1 commit into
Closed
fix(utils): allow negative POSIX timestamps in from_timestamp()#3020Sohel2309 wants to merge 1 commit into
Sohel2309 wants to merge 1 commit into
Conversation
from_timestamp() explicitly rejected any negative value with
"Not a valid POSIX timestamp", but negative POSIX timestamps are
valid and represent real dates before 1970-01-01. Since
fields.DateTime(format="timestamp"/"timestamp_ms") uses
datetime.timestamp() to serialize -- which correctly produces a
negative float for pre-epoch datetimes -- the field could serialize
a value that it would then refuse to deserialize, breaking the
serialize/deserialize round trip for any date before the Unix epoch:
field = fields.DateTime(format="timestamp")
serialized = field.serialize("d", {"d": datetime(1950, 5, 3, tzinfo=utc)})
# -620568000.0 -- correct
field.deserialize(serialized)
# ValidationError: Not a valid datetime. -- should round-trip
Confirmed the underlying stdlib call already handles negative values
correctly (datetime.fromtimestamp(-620568000.0, tz=utc) round-trips
exactly), so the explicit "if value < 0" guard was not protecting
against any real platform limitation -- the existing OSError/
OverflowError handling already covers genuine conversion failures.
This is a continuation of the fix for marshmallow-code#2133 (timestamp 0 was
previously rejected the same way and was fixed to be allowed); the
broader range of legitimate negative timestamps was left unhandled.
Fix: remove the "if value < 0" check in from_timestamp() (utils.py).
from_timestamp_ms() delegates to from_timestamp() and is fixed by the
same change.
Tests:
- Fixed test_from_timestamp_with_negative_value and
test_invalid_timestamp_field_deserialization, which had encoded the
buggy rejection as expected behavior.
- Added negative-value cases to the existing
test_from_timestamp/test_timestamp_field_deserialization parametrized
tables.
- Added test_timestamp_field_deserialization_round_trip_negative,
which serializes real pre-1970 datetimes (no mocks) and asserts
deserializing the result reproduces the original value -- this is
the direct regression test for the round-trip bug. Verified it (and
the negative-value parametrize cases) fail on the pre-fix code with
ValidationError: Not a valid datetime., and pass after the fix.
Member
|
Replaced by #3026. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #3019.
from_timestamp()currently rejects negative POSIX timestamps even though negative timestamps are valid and are produced byfields.DateTime(format="timestamp")when serializing datetimes before the Unix epoch.This breaks serialization/deserialization round-tripping for valid pre-1970 datetimes.
Reproduction
The same field can therefore produce a value that it cannot deserialize.
Root cause
from_timestamp()explicitly rejected values below zero before callingdatetime.fromtimestamp().Python's timestamp conversion already supports negative POSIX timestamps, so this explicit restriction was unnecessarily preventing valid dates from being deserialized.
Fix
Remove the explicit negative-value rejection and rely on the existing timestamp conversion and
OSError/OverflowErrorhandling for genuine conversion failures.Regression coverage was added for negative timestamps, including timestamp and timestamp-millisecond formats.
Validation
Fixes #3019